Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.#352
Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.#352alpharius-ck wants to merge 41 commits into
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
# Conflicts: # turbo.json
Co-authored-by: Cursor <cursoragent@cursor.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
alpharius-ck
changed the title
hurali97
left a comment
hurali97
left a comment
There was a problem hiding this comment.
Great work. As explained in the comments, we should add e2e testing behavior for a brownfield workflow and not greenfield. Most of the changes in the PR are unnecessary as they try to fix greenfield and while in a real project environment that might makes sense, in case of this repo, we do not need it.
Our main goal should be to verify AppleApp and AndroidApp works in e2e testing with brownfield packages.
|
One comment from my end: can we merge the E2E workflow with the |
# Conflicts: # apps/AppleApp/Brownfield Apple App.xcodeproj/project.pbxproj # apps/AppleApp/Brownfield Apple App/components/ContentView.swift # apps/AppleApp/prepareXCFrameworks.js
| - name: Install pods (RN ${{ inputs.variant }} app, E2E) | ||
| if: inputs.variant == 'vanilla' && inputs.run-e2e == 'true' | ||
| env: | ||
| RCT_USE_PREBUILT_RNCORE: '0' |
There was a problem hiding this comment.
Q: Does using prebuilt causes error? I believe if we use prebuilts, we might save some time on CI?
| GreetingCard(name: hostAppName) | ||
|
|
||
| MessagesView() | ||
| MessagesView() |
| .frame(maxWidth: .infinity) | ||
| .padding(16) | ||
| } | ||
| if showPostMessageToast { |
There was a problem hiding this comment.
This file appears to be out of format.
| } | ||
| } | ||
|
|
||
| struct DetoxE2eButton: UIViewRepresentable { |
There was a problem hiding this comment.
Why do we need a UIKit bridging for a button in SwiftUI? I believe we can do all this in SwiftUI unless you're trying for a specific use case?
There was a problem hiding this comment.
Agreed — switched to SwiftUI .accessibilityIdentifier() and removed the UIViewRepresentable wrappers.
| DetoxE2E: 'YES', | ||
| }; | ||
|
|
||
| async function scrollToEmbeddedRn() { |
There was a problem hiding this comment.
There are similar functions in this file and apps/brownfield-example-shared-tests/e2e/appleAppBrownfield.e2e.js - perhaps these can be moved to utils or similar?
There was a problem hiding this comment.
These scripts share almost the same code, perhaps you can create a base script which these 4 can invoke with params
There was a problem hiding this comment.
Q: Why are these placed in this package and not where they might be used? I might be missing some context.
There was a problem hiding this comment.
The test covers the Expo config plugin (withDetoxEmbeddedBundleIosAppDelegate.ts) that patches AppDelegate during expo prebuild. The plugin lives in packages/react-native-brownfield, so the unit test belongs next to the implementation — same pattern as other config-plugin tests. It validates the Swift AppDelegate patch logic (embedded bundle fallback for -BrownfieldPreferEmbeddedBundleInDebug), not app runtime behavior.
| `; | ||
| } | ||
|
|
||
| function collectTypesUsedByMethods(methods: MethodSignature[]): Set<string> { |
There was a problem hiding this comment.
Q: why are these changes required?
There was a problem hiding this comment.
Navigation codegen was emitting import type { SomeModel } from './NativeBrownfieldNavigation' for all referenced type declarations, even when methods map those models to Object / ReadableMap in the generated TurboModule spec. That produced unused type imports in generated index.ts / index.d.ts, which broke TypeScript/eslint in Expo apps using brownfield navigation.
collectTypesUsedByMethods filters imports to types actually referenced in generated method signatures.
|
From my end, LGTM after 1) CI green 2) Hur's comment addressed. Good job here! |
5 participants
Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.
Introduces a reusable composite action (.github/actions/e2e-ios) that:
Test plan
If a job fails, download the detox-rnapp-ios / detox-expo54-ios / detox-expo55-ios artifact and inspect screenshots/logs
Changes for native files:
BrownfieldStore “not found” fix (RNApp iOS)
This document summarizes the investigation and fixes for the Brownie error when running RNApp on iOS:
AppleApp (brownfield host) worked with the same JS and store schema; only standalone RNApp failed until these changes were applied.
The problem
When running RNApp on iOS, React crashed on the first render of
Counterwith the error above.useStore('BrownfieldStore', …)in JavaScript could not see a store that native code was supposed to have registered.How Brownie works (short)
Brownie shares state between native and React Native through two layers:
BrownieStoreManager— holds stores and backsglobal.__brownieGetStore(what JS uses).Store/StoreManager— optional native-side wrapper;BrownfieldStore.register(...)creates aStoreand calls into the C++ bridge.JavaScript (
@callstack/brownie) reads stores via:That global is installed by
BrownieInstaller::install(), which must run when the Brownie turbo module is loaded under the New Architecture.Native registration must hit the same C++ manager instance that JavaScript uses.
Root causes
There were two separate issues, not one.
1. Duplicate Brownie in RNApp (main RNApp bug)
RNApp linked and embedded
BrownfieldLib.frameworkinto the app. That framework was built withinherit! :completein the Podfile, so it contained a full second copy of Brownie (including its ownBrownieStoreManagersingleton).At runtime:
AppDelegate→BrownfieldStore.register(...)__brownieGetStore→ C++ lookupRegistration and JS were talking to different singletons, so JS always saw “store not found” even though registration appeared to succeed on native.
AppleApp avoids this: it links one
Brownie.xcframeworkand oneBrownfieldLib.xcframeworkseparately, and registers viaimport Browniein the app — not by embedding a BrownfieldLib that re-exports and duplicates Brownie.2. JSI bindings not installed on iOS New Architecture (brownie package bug)
On iOS with
RCT_NEW_ARCH_ENABLED, React installs turbo-module JSI bindings only when the module is a C++TurboModuleWithJSIBindings.BrownieModuleonly implemented the ObjC protocolRCTTurboModuleWithJSIBindings. In bridgeless / New Architecture, that path is not used;dynamic_cast<TurboModuleWithJSIBindings*>on the C++ module fails, soBrownieInstaller::install()never ran andglobal.__brownieGetStorestayed undefined.Android already handled this in
BrownieModule.initialize()viainstallJSIBindingsIfNeeded().This could affect any iOS React Native 0.85+ app using Brownie; RNApp made it obvious because JS hits the store immediately on launch.
What we changed
RNApp — iOS app target
ios/RNApp.xcodeproj/project.pbxprojBrownfieldLibfrom the RNApp app target. TheBrownfieldLibtarget remains forbrownfield package:ios.ios/RNApp/AppDelegate.swiftimport Brownie; register store beforefactory.startReactNative(...)viaBrownieBootstrap.register(...).ios/BrownfieldLib/BrownfieldLib.swift@_exported import Brownie. Only re-exportsReactBrownfield.BrownfieldLibRNApp — Android
android/app/src/main/java/com/rnapp/MainApplication.ktregisterStoreIfNeeded(...)forBrownfieldStorebeforeloadReactNative(this).android/app/build.gradleimplementation(project(":BrownfieldLib"))for generated Kotlin types.@callstack/browniepackagepackages/brownie/ios/BrownieModule.mmBrownieTurboModuleextendingNativeBrownieModuleSpecJSI+TurboModuleWithJSIBindings;getTurboModulereturns it;installJSIBindingsWithRuntimecallsBrownieInstaller::install(runtime).__brownieGetStorenever set on New Architecturepackages/brownie/ios/BrownieModule.hRCTTurboModuleWithJSIBindingsimport from header.packages/brownie/ios/BrownieStore.swiftBrownieBootstrap— registers viaBrownieStoreBridgedirectly (C++ path JS uses).RNApp — tooling
package.jsonios/androidscripts runyarn codegenfirst.Codegen (operational)
From
apps/RNApp:This generates:
packages/brownie/ios/Generated/BrownfieldStore.swift(gitignored)android/BrownfieldLib/.../Generated/BrownfieldStore.ktWhy each fix works
Stop embedding
BrownfieldLibin RNAppRNApp as a standalone React Native app does not need the brownfield packaging framework at runtime. Embedding it pulled in a second Brownie. After removal, there is one
BrownieStoreManagerin the process; registration and JS use the same store map.BrownfieldLibis still built foryarn brownfield:package:ios; it is just not part of the dev app binary anymore.Register before React Native starts
Brownie expects stores to exist before the JS bundle runs components that call
useStore. BothAppDelegate(iOS) andMainApplication(Android) register initial state before starting RN.BrownieTurboModule(library fix)When JS first loads the Brownie turbo module, React calls
TurboModuleWithJSIBindings::installJSIBindingson the C++ module. That runsBrownieInstaller::install, which definesglobal.__brownieGetStore. Without this, JS fails even with a correctly registered native store.BrownieBootstrap(optional)BrownfieldStore.registercreates a SwiftStoreand also talks to the bridge. After the duplicate was removed,BrownfieldStore.registeris sufficient for RNApp again.BrownieBootstrapregisters straight throughBrownieStoreBridgeand documents “use this from AppDelegate before RN starts.” You can use either:No Brownie re-export in
BrownfieldLibPackaged
BrownfieldLib.xcframeworkshould not embed another full Brownie. Host apps (like AppleApp) link Brownie explicitly. That keeps brownfield packaging aligned with AppleApp’s working layout.Required vs optional going forward
Required for RNApp
BrownfieldLibin the RNApp app target.BrownieTurboModulein@callstack/browniefor iOS New Architecture.yarn codegenwhen store schemas change.Optional
BrownieBootstrapvsBrownfieldStore.registerinAppDelegate(equivalent after duplicate fix).yarn codegen &&prepended toios/androidscripts.Unchanged
BrownfieldStore.registerin appinit, separateBrownie+BrownfieldLibxcframeworks — no change needed.End-to-end flow after fixes (RNApp iOS)
sequenceDiagram participant AD as AppDelegate participant BB as Brownie C++ manager participant RN as React Native participant JS as JS useStore AD->>BB: BrownieBootstrap / register store AD->>RN: startReactNative RN->>JS: Load bundle JS->>RN: Require Brownie turbo module RN->>BB: BrownieTurboModule installs __brownieGetStore JS->>BB: __brownieGetStore("BrownfieldStore") BB-->>JS: Host object with stateTakeaway
The error looked like “forgot to register the store,” but RNApp actually had two separate issues:
BrownfieldLib) — why AppleApp worked and RNApp did not.BrownieTurboModule.Removing the duplicate was the decisive fix for RNApp; the turbo module fix is still necessary for correct Brownie behavior on modern React Native iOS.